Skip to content

Chunk download latency #634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 22 commits into from
Jul 21, 2025
Merged

Chunk download latency #634

merged 22 commits into from
Jul 21, 2025

Conversation

saishreeeee
Copy link
Collaborator

@saishreeeee saishreeeee commented Jul 13, 2025 •
edited
Loading

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • Other

Description

Record chunk download latency
Added chunk_id to SqlExecutionEvent and ResultDownloadHandler

How is this tested?

  • Unit tests
  • E2E Tests
  • Manually
  • N/A

Ran the query: SELECT * FROM RANGE(20000000)
Latency was recorded for 4 chunks.
The latency log for the 1st chunk:

{
  "frontend_log_event_id": "e684ff49-9026-49af-abb6-687a6a16e565",
  "context": {
    "client_context": {
      "timestamp_millis": 1752467452769,
      "user_agent": "PyDatabricksSqlConnector/4.0.5"
    }
  },
  "entry": {
    "sql_driver_log": {
      "session_id": "01f0606b-4825-118d-abd2-87f5748c7dd3",
      "system_configuration": {
        "driver_version": "4.0.5",
        "os_name": "Darwin",
        "os_version": "24.5.0",
        "os_arch": "arm64",
        "runtime_name": "Python 3.13.3",
        "runtime_version": "3.13.3",
        "runtime_vendor": "CPython",
        "driver_name": "Databricks SQL Python Connector",
        "char_set_encoding": "utf-8",
        "locale_name": "en_US"
      },
      "driver_connection_params": {
        "http_path": <HTTP_PATH>,
        "mode": "THRIFT",
        "host_info": {
          "host_url": <SERVER_HOSTNAME>,
          "port": 443
        },
        "auth_mech": "PAT"
      },
      "sql_statement_id": "01f0606b-4854-1f89-a0cb-d5860332ef81",
      "sql_operation": {
        "statement_type": "QUERY",
        "is_compressed": true,
        "execution_result": "EXTERNAL_LINKS",
        "chunk_id": 0
      },
      "operation_latency_ms": 15882
    }
  }
}

Related Tickets & Documents

PECOBLR-653

@saishreeeee
chunk download latency
928e128 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
formatting
804f9e0 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
test fixes
e111909 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
check types fix
e84ba7d 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
type fix revert
c054039 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
-
3e0bb1e 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee saishreeeee marked this pull request as ready for review July 14, 2025 04:46
@saishreeeee saishreeeee self-assigned this Jul 14, 2025
@saishreeeee saishreeeee requested a review from jprakash-db July 14, 2025 04:46
vikrantpuppala
vikrantpuppala reviewed Jul 14, 2025
View reviewed changes
@saishreeeee
removed result set extractor
5345384 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
databricks client type
a0318e3 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
formatting
e79c325 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
added statement type to command id
1b9a2b8 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
check types fix
153d346 
Signed-off-by: Sai Shree Pradhan <[email protected]>
vikrantpuppala
vikrantpuppala approved these changes Jul 16, 2025
View reviewed changes
@vikrantpuppala
Copy link
Contributor

@jayantsing-db can you take a look, this PR adds latency logs (merges into sea-migration) as there are some refactorings that this PR leverages

jprakash-db
jprakash-db reviewed Jul 16, 2025
View reviewed changes
@saishreeeee
merge sea-migration fixes
2564d41 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee
comment fix
d78ffba 
Signed-off-by: Sai Shree Pradhan <[email protected]>
@saishreeeee saishreeeee requested a review from jprakash-db July 16, 2025 10:25
jprakash-db
jprakash-db approved these changes Jul 17, 2025
View reviewed changes
Copy link
Contributor

@jprakash-db jprakash-db left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for making the changes

varun-edachali-dbx
varun-edachali-dbx reviewed Jul 21, 2025
View reviewed changes
@saishreeeee
removed dup check for trowset
6896f93 
Signed-off-by: Sai Shree Pradhan <[email protected]>
varun-edachali-dbx
varun-edachali-dbx approved these changes Jul 21, 2025
View reviewed changes
Copy link
Collaborator

@varun-edachali-dbx varun-edachali-dbx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@saishreeeee saishreeeee merged commit b57c3f3 into sea-migration Jul 21, 2025
23 checks passed
varun-edachali-dbx added a commit that referenced this pull request Jul 22, 2025
@varun-edachali-dbx
Revert "Chunk download latency (#634)"
77c0343 
This reverts commit b57c3f3.
@varun-edachali-dbx varun-edachali-dbx mentioned this pull request Jul 23, 2025
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vikrantpuppala vikrantpuppala vikrantpuppala approved these changes

@jprakash-db jprakash-db jprakash-db approved these changes

@varun-edachali-dbx varun-edachali-dbx varun-edachali-dbx approved these changes

@jayantsing-db jayantsing-db Awaiting requested review from jayantsing-db

Assignees

@saishreeeee saishreeeee

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@saishreeeee @vikrantpuppala @jprakash-db @varun-edachali-dbx